Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

flush IPv6 rules #181

Closed
wants to merge 5 commits into from
Closed

flush IPv6 rules #181

wants to merge 5 commits into from

Conversation

PaulusTM
Copy link
Contributor

This PR fixes #180

This is my first PR into a Chef repo, if something is not right, please advise :)

If I converge a default-centos-72 instance and converge it I have the following output

[root@default-centos-72 ~]# cat /etc/sysconfig/firewalld-chef.rules | grep OUTPUT
firewall-cmd --direct --add-rule ipv4 filter OUTPUT 50 -p tcp -m tcp -m multiport --dports 443,80 -m comment --comment 'HTTP HTTPS' -j ACCEPT
firewall-cmd --permanent --direct --add-rule ipv4 filter OUTPUT 50 -p tcp -m tcp -m multiport --dports 443,80 -m comment --comment 'HTTP HTTPS' -j ACCEPT
firewall-cmd --direct --add-rule ipv6 filter OUTPUT 50 -p tcp -m tcp -m multiport --dports 443,80 -m comment --comment 'HTTP HTTPS' -j ACCEPT
firewall-cmd --permanent --direct --add-rule ipv6 filter OUTPUT 50 -p tcp -m tcp -m multiport --dports 443,80 -m comment --comment 'HTTP HTTPS' -j ACCEPT

[root@default-centos-72 ~]# firewall-cmd --direct --get-all-rules | grep OUTPUT
ipv6 filter OUTPUT 50 -p tcp -m tcp -m multiport --dports 443,80 -m comment --comment 'HTTP HTTPS' -j ACCEPT
ipv4 filter OUTPUT 50 -p tcp -m tcp -m multiport --dports 443,80 -m comment --comment 'HTTP HTTPS' -j ACCEPT

If I then comment out firewall_rule 'HTTP HTTPS'
https://github.com/chef-cookbooks/firewall/blob/master/test/fixtures/cookbooks/firewall-test/recipes/default.rb#L109-L114
And re-converge the VM

[root@default-centos-72 ~]# cat /etc/sysconfig/firewalld-chef.rules | grep OUTPUT

[root@default-centos-72 ~]# firewall-cmd --direct --get-all-rules | grep OUTPUT

@PaulusTM
also flush IPv6 rules
7bcaa6b 
Signed-off-by: Daniel Paulus <[email protected]>
martinb3
martinb3 reviewed Oct 11, 2017
View reviewed changes
Copy link
Contributor

@martinb3 martinb3 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this change! I only made one note, just so we don't break anyone who has ipv6 disabled. If you wouldn't mind revising that, I think this is good to merge. Thanks again!

libraries/helpers_firewalld.rb Outdated
@@ -39,6 +39,8 @@ def firewalld_flush!

shell_out!('firewall-cmd', '--direct', '--remove-rules', 'ipv4', 'filter', 'INPUT')
shell_out!('firewall-cmd', '--direct', '--remove-rules', 'ipv4', 'filter', 'OUTPUT')
shell_out!('firewall-cmd', '--direct', '--remove-rules', 'ipv6', 'filter', 'INPUT')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add a guard for ipv6_enabled?(new_resource)? Just so we don't run this if ipv6 is disabled? Or confirm that it won't blow up if ipv6 isn't present/enabled on the system?

@iennae
Copy link

iennae commented Dec 8, 2017

@dpnl87 Thanks for submitting this PR. Could you take a look at the changes @martinb3 has requested? Thanks! We'd love to accept your first PR :)

@PaulusTM
Copy link
Contributor Author

I'll have a look at the change next week when my Holiday starts. Thanks for the patience :)

@PaulusTM
Copy link
Contributor Author

PaulusTM commented Jan 2, 2018

@martinb3 I've add the guard. Is this in line with your suggestion?

This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@martinb3 martinb3 martinb3 left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Removing firewall_rule from recipe does not remove ipv6 rule from kernel
5 participants
@PaulusTM @iennae @martinb3 @tas50 @xorimabot